Skip to content

Conversation

@jakemas
Copy link
Contributor

@jakemas jakemas commented Oct 8, 2025

This PR adds mldsa_native.h which is a prerequisite for the monobuilds.

@jakemas jakemas marked this pull request as ready for review October 8, 2025 20:00
@jakemas jakemas requested a review from a team as a code owner October 8, 2025 20:00
Copy link
Contributor

@hanno-becker hanno-becker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jakemas IIRC the plan was to have the monobuild files in the monobuild example folder first, and eventually restructure everything to follow the directory pattern as in mlkem-native (with mldsa/src/* for the bulk of the source, and mldsa/mldsa_native.* for the toplevel). I don't think we want mldsa_native.c in the toplevel directory.

Why is mldsa_native.c needed for this change at all, actually?

@mkannwischer
Copy link
Contributor

@jakemas - this is blocking the alpha release. Are you planning to finish this soon, or do you want me to take over?

@mkannwischer mkannwischer force-pushed the mldsa-native-api-header branch 2 times, most recently from 20b9f11 to 0b74d9a Compare October 31, 2025 11:55
@mkannwischer
Copy link
Contributor

@jakemas - this is blocking the alpha release. Are you planning to finish this soon, or do you want me to take over?

I took the liberty to finish this up.

@mkannwischer mkannwischer force-pushed the mldsa-native-api-header branch 3 times, most recently from 35e4daa to b102e39 Compare November 1, 2025 12:17
This was referenced Nov 1, 2025
@mkannwischer mkannwischer force-pushed the mldsa-native-api-header branch from b102e39 to a6a4882 Compare November 2, 2025 02:30
Copy link
Contributor

@hanno-becker hanno-becker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two comments on the use of the enum and consistency of array vs. pointer declarations. Both can be follow-ups. Thanks @mkannwischer!

Co-authored-by: Matthias J. Kannwischer <[email protected]>
Signed-off-by: Jake Massimo <[email protected]>
Signed-off-by: Matthias J. Kannwischer <[email protected]>
@mkannwischer mkannwischer force-pushed the mldsa-native-api-header branch from a6a4882 to 070d7bd Compare November 2, 2025 05:36
@mkannwischer mkannwischer merged commit ee7e1bf into main Nov 2, 2025
247 checks passed
@mkannwischer mkannwischer deleted the mldsa-native-api-header branch November 2, 2025 06:37
mkannwischer added a commit that referenced this pull request Nov 4, 2025
Currently we have an enum for the 12 different pre-hash functions for
HashML-DSA. This leads to problems in multi-level builds as we must only
define the enum once. Currently we work around this by guarding the enum
definition with a pre-processor conditional.
However, there is also a (theoretical) concern about the type of the enum
being implementation-defined in C90:
#537 (comment)

It seems cleaner to not use an enum here, but instead use #defines avoiding all
the above problems.
This commit implements that change.

We also eliminate the camel case hashAlg - that was inconsistent with the
remaining code base from the start.

Resolves
#591

Signed-off-by: Matthias J. Kannwischer <[email protected]>
mkannwischer added a commit that referenced this pull request Nov 4, 2025
Currently we have an enum for the 12 different pre-hash functions for
HashML-DSA. This leads to problems in multi-level builds as we must only
define the enum once. Currently we work around this by guarding the enum
definition with a pre-processor conditional.
However, there is also a (theoretical) concern about the type of the enum
being implementation-defined in C90:
#537 (comment)

It seems cleaner to not use an enum here, but instead use #defines avoiding all
the above problems.
This commit implements that change.

We also eliminate the camel case hashAlg - that was inconsistent with the
remaining code base from the start.

Resolves
#591

Signed-off-by: Matthias J. Kannwischer <[email protected]>
mkannwischer added a commit that referenced this pull request Nov 4, 2025
Currently we have an enum for the 12 different pre-hash functions for
HashML-DSA. This leads to problems in multi-level builds as we must only
define the enum once. Currently we work around this by guarding the enum
definition with a pre-processor conditional.
However, there is also a (theoretical) concern about the type of the enum
being implementation-defined in C90:
#537 (comment)

It seems cleaner to not use an enum here, but instead use #defines avoiding all
the above problems.
This commit implements that change.

We also eliminate the camel case hashAlg - that was inconsistent with the
remaining code base from the start.

Resolves
#591

Signed-off-by: Matthias J. Kannwischer <[email protected]>
mkannwischer added a commit that referenced this pull request Nov 5, 2025
Currently we have an enum for the 12 different pre-hash functions for
HashML-DSA. This leads to problems in multi-level builds as we must only
define the enum once. Currently we work around this by guarding the enum
definition with a pre-processor conditional.
However, there is also a (theoretical) concern about the type of the enum
being implementation-defined in C90:
#537 (comment)

It seems cleaner to not use an enum here, but instead use #defines avoiding all
the above problems.
This commit implements that change.

We also eliminate the camel case hashAlg - that was inconsistent with the
remaining code base from the start.

Resolves
#591

Signed-off-by: Matthias J. Kannwischer <[email protected]>
hanno-becker pushed a commit that referenced this pull request Nov 5, 2025
Currently we have an enum for the 12 different pre-hash functions for
HashML-DSA. This leads to problems in multi-level builds as we must only
define the enum once. Currently we work around this by guarding the enum
definition with a pre-processor conditional.
However, there is also a (theoretical) concern about the type of the enum
being implementation-defined in C90:
#537 (comment)

It seems cleaner to not use an enum here, but instead use #defines avoiding all
the above problems.
This commit implements that change.

We also eliminate the camel case hashAlg - that was inconsistent with the
remaining code base from the start.

Resolves
#591

Signed-off-by: Matthias J. Kannwischer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update api.h to reflect all supported API functions (and rename to mldsa_native.h)

4 participants